-
Notifications
You must be signed in to change notification settings - Fork 37
Support Let's Encrypt for Open OnDemand #714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c346577
to
2136629
Compare
a85f8da
to
ffeafae
Compare
ffeafae
to
ccafe06
Compare
|
||
Alternatively, you can generate a certificate from Let's Encrypt automatically by configuring the following variables: | ||
- `openondemand_certbot`: Optional. Default is false. Set to true to request a certificate from Let's Encrypt. | ||
- `openondemand_certbot_email`: Optional. Default is empty. Set to the admin email address if using Let's Encrypt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the domain for this have to match e.g. the cluster_domain_suffix or anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few refactoring comments. Want to work out a way merging this doens't force everyone to add HTTP secgroup too though - maybe it is just docs in the role?
default = [ | ||
"default", # allow all in-cluster services | ||
"SSH", # access via ssh | ||
"HTTP", # HTTP-01 challenge and redirect to HTTPS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like the fact this means all existing deployments, when they merge this, are either going to have to add this HTTP secgroup or else override login_security_groups
(or equivalent for the nodegroup containing ondemand host). I wonder if we can work around this somehow...
import_role: | ||
name: openondemand | ||
tasks_from: certbot.yml | ||
when: "'openondemand' in group_names" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should also add when: openondemand_certbot | bool
for consistency with behaviour of openondemand/tasks/main.yml
?
|
||
- name: Generate Let's Encrypt certificate | ||
command: sudo certbot certonly --standalone -d {{ openondemand_servername }} -n -m {{ openondemand_certbot_email }} --agree-tos | ||
when: appliances_mode == 'configure' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be more consistent as:
when: appliances_mode == 'configure' | |
when: appliances_mode != 'build' |
b/c you can use e.g. -e appliances_mode=all
to fake a build + deploy for debugging/dev - generally we've tried to make it so that not using "build" or "configure" does "everything".
But its definitely not totally consistent so not very fussed.
- `openondemand_ssl_cert_key`: Optional. Default `/etc/pki/tls/private/localhost.key` (unless `openondemand_certbot` is true). | ||
|
||
Alternatively, you can generate a certificate from Let's Encrypt automatically by configuring the following variables: | ||
- `openondemand_certbot`: Optional. Default is false. Set to true to request a certificate from Let's Encrypt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree this is the correct default, but wonder if we should set it true
in stackhpc so
a) we include these packages in the StackHPC images
b) CI tests certbot
- Created instances have access to internet (note proxies can be setup through the appliance if necessary). | ||
- Created instances have accurate/synchronised time (for VM instances this is usually provided by the hypervisor; if not or for bare metal instances it may be necessary to configure a time service via the appliance). | ||
- Three security groups are present: `default` allowing intra-cluster communication, `SSH` allowing external access via SSH and `HTTPS` allowing access for Open OnDemand. | ||
- Four security groups are present: ``default`` allowing intra-cluster communication, ``SSH`` allowing external access via SSH, and ``HTTP`` and ``HTTPS`` allowing access for Open OnDemand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
- python3-certbot-apache | ||
|
||
- block: | ||
- name: Validate that server name is set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually already done in ansible/roles/openondemand/tasks/validate.yml?
- certbot | ||
- python3-certbot-apache | ||
|
||
- block: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move these to ansible/roles/openondemand/tasks/validate.yml? That is called much earlier (buy ansible/validate) than this, and only from site.yml so doesn't run during build.
No description provided.